feat(sdks): add Node and Java parity debt#437
Conversation
Python has had both; this SDK had neither, so a completed job could not be read back at all — terminal jobs are archived as they finish, so the archive is the only place they live. Both follow the existing scan-method shape (async + spawn_blocking). The wider filter takes its own input type, since it matches on metadata/error substrings and a created-at range that the narrow one has no fields for; the status parsing they share is now one helper rather than two copies.
Every SDK pages the same way, and "fewer rows than limit means last page" is part of the cursor contract rather than of any one shell — leaving it in the Python binding would have had the other two copy it. Pure move; Python now imports it.
Python has had cursor paging since Tier 2; the core methods were already there. Adds listJobsAfter, listJobsFilteredAfter, listArchivedAfter and deadLettersAfter, so a deep listing stays O(page) instead of O(offset). napi has no generics, so each item type gets its own page object and the shell declares one Page<T> they satisfy structurally. It also types an Option as optional but hands back null, so the shell normalizes: nextCursor is null on the last page, matching the other SDKs, and a `=== undefined` check against the raw value would never have fired.
Python has had cursor paging since Tier 2 and the core methods were already there. Adds listJobsAfter and listArchivedAfter with a Page<T>, so a deep listing stays O(page) rather than O(offset). The SPI methods are default: QueueBackend has an implementor outside this module, and an abstract method would break it and every downstream backend. They throw rather than return an empty page — a backend that cannot seek would otherwise look like it had reached the last page. The in-memory test backend implements them for real, since a default-only method is untested API. The smoke exercises a paginated call: GraalVM metadata is generated from what runs, so a DTO it never touches gets no reflection entry and fails under --no-fallback at runtime rather than in CI.
Python and Node can turn a middleware off for one task without a redeploy; this SDK had only the global use(). Adds disableMiddleware/enableMiddleware/ listDisabledMiddleware over the existing settings, keyed the same way as the other SDKs. The list is read per invocation rather than cached, like Node, so a toggle takes effect on the next job with no restart. The chain is resolved once and reused for before/after/onError: re-reading between them would let a toggle landing mid-job run after on a middleware whose before never ran. The key format and the name derivation live in one place, so the worker that honours the list and the API that writes it cannot drift apart.
📝 WalkthroughWalkthroughThe PR adds keyset-pagination cursor and page APIs across Rust, Java, and Node, including filtering, archived/dead-letter listings, cursor validation, and tests. It also adds Java per-task middleware disable, enable, and listing operations integrated into worker dispatch. ChangesKeyset pagination
Per-task middleware controls
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Queue
participant Storage
Client->>Queue: listJobsAfter(filter, after)
Queue->>Storage: query rows after cursor
Storage-->>Queue: page of jobs
Queue-->>Client: items and nextCursor
sequenceDiagram
participant Taskito
participant QueueBackend
participant WorkerDispatchBridge
Taskito->>QueueBackend: persist disabled middleware
WorkerDispatchBridge->>QueueBackend: read task settings
QueueBackend-->>WorkerDispatchBridge: disabled middleware names
WorkerDispatchBridge->>WorkerDispatchBridge: execute resolved middleware chain
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdks/java/src/main/java/org/byteveda/taskito/worker/WorkerDispatchBridge.java (1)
138-149: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReuse the invocation’s middleware chain for outcome callbacks.
Line 142 re-reads disable state, so a mid-job toggle can suppress
onCompletedfor middleware that ranbefore, or invoke it on middleware that was skipped. Retain the chain resolved inrunJoband use that same chain for the corresponding outcome; add a mid-flight-toggle regression test covering the outcome hook.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/main/java/org/byteveda/taskito/worker/WorkerDispatchBridge.java` around lines 138 - 149, Update WorkerDispatchBridge.runJob and onOutcome to retain the middleware chain selected when the job starts and pass that same chain to the corresponding outcome callback, instead of re-resolving via disables.resolve(taskName, middleware). Preserve the existing dispatch and error-isolation behavior, and add a regression test that toggles middleware mid-flight to verify outcome hooks match the middleware that ran before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/taskito-node/src/queue/inspect.rs`:
- Around line 182-239: Validate the filter’s offset before entering the
keyset-pagination flow in both list_jobs_after and list_jobs_filtered_after.
Reject any non-zero offset with the existing argument-validation error
mechanism, while allowing zero or absent offsets and preserving the current
cursor-based behavior.
In `@sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java`:
- Around line 1070-1085: Update disableMiddleware and enableMiddleware to
perform read-modify-write updates through the backend’s atomic mutation or
CAS-retry mechanism for MiddlewareDisables.key(taskName). Ensure concurrent
additions and removals are retried or serialized so neither operation overwrites
another’s change, while preserving the existing duplicate-check and removal
behavior.
In `@sdks/java/src/main/java/org/byteveda/taskito/model/Page.java`:
- Around line 14-20: Update the pagination example in Page.java to call
listJobsAfter with only the JobFilter and cursor arguments. Configure the page
size of 50 on the JobFilter before entering the loop, while preserving the
existing cursor iteration and item handling.
In
`@sdks/java/src/main/java/org/byteveda/taskito/worker/WorkerDispatchBridge.java`:
- Around line 96-111: Move the disables.resolve(taskName, middleware) call into
the protected try block in the worker dispatch flow, keeping the resulting chain
available to both before/after and error handling. Ensure exceptions during
chain resolution enter the existing catch path so the job is resolved and
cleanup behavior is preserved.
In
`@sdks/java/test-support/src/main/java/org/byteveda/taskito/test/InMemoryQueueBackend.java`:
- Around line 219-239: Normalize negative pagination limits before calling
keysetPage in listJobsAfterJson: clamp the decoded int limit to zero. Apply the
same normalization in the sibling long-limit path at
sdks/java/test-support/src/main/java/org/byteveda/taskito/test/InMemoryQueueBackend.java
lines 243-254, before capping and casting to int, so negative values produce an
empty page without subList errors.
---
Outside diff comments:
In
`@sdks/java/src/main/java/org/byteveda/taskito/worker/WorkerDispatchBridge.java`:
- Around line 138-149: Update WorkerDispatchBridge.runJob and onOutcome to
retain the middleware chain selected when the job starts and pass that same
chain to the corresponding outcome callback, instead of re-resolving via
disables.resolve(taskName, middleware). Preserve the existing dispatch and
error-isolation behavior, and add a regression test that toggles middleware
mid-flight to verify outcome hooks match the middleware that ran before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca822abf-a01c-41a1-bbed-ac98087bfebf
📒 Files selected for processing (28)
crates/taskito-core/src/storage/cursor.rscrates/taskito-java/src/convert.rscrates/taskito-java/src/queue/inspect.rscrates/taskito-node/src/config.rscrates/taskito-node/src/convert/job.rscrates/taskito-node/src/convert/mod.rscrates/taskito-node/src/convert/stats.rscrates/taskito-node/src/queue/admin.rscrates/taskito-node/src/queue/inspect.rscrates/taskito-python/src/py_queue/mod.rssdks/java/graalvm-smoke/src/main/java/org/byteveda/taskito/graalvm/Smoke.javasdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.javasdks/java/src/main/java/org/byteveda/taskito/Taskito.javasdks/java/src/main/java/org/byteveda/taskito/internal/JniQueueBackend.javasdks/java/src/main/java/org/byteveda/taskito/internal/MiddlewareDisables.javasdks/java/src/main/java/org/byteveda/taskito/internal/NativeQueue.javasdks/java/src/main/java/org/byteveda/taskito/model/Page.javasdks/java/src/main/java/org/byteveda/taskito/spi/QueueBackend.javasdks/java/src/main/java/org/byteveda/taskito/worker/WorkerDispatchBridge.javasdks/java/src/test/java/org/byteveda/taskito/core/PaginationTest.javasdks/java/src/test/java/org/byteveda/taskito/worker/MiddlewareDisableTest.javasdks/java/test-support/src/main/java/org/byteveda/taskito/test/InMemoryQueueBackend.javasdks/node/src/index.tssdks/node/src/native.tssdks/node/src/queue.tssdks/node/src/types.tssdks/node/test/core/listings.test.tssdks/node/test/core/pagination.test.ts
Resolving reads the backend, so it can throw — and it ran after the resource scope was bound but before the try, leaving the scope entered, the finally unreached and the job neither completed nor failed until the reaper timed it out. A settings failure now fails the job like any other. The chain starts empty so that failure runs onError on nothing, which is right: no before() ran either.
The keyset methods took the offset-paged filter and ignored its offset, so a caller passing one got a different page than they asked for, silently. Offset and cursor paging do not compose — a cursor already says where to resume. The TS filters omit the field, and the native call rejects a non-zero one for a caller who reaches past the type.
subList would throw on a negative limit where the native handlers clamp it to an empty page, so the in-memory backend diverged from the contract it exists to stand in for. The Page javadoc called listJobsAfter with a limit argument it does not take.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdks/node/src/types.ts (1)
178-188: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAccept
nullfor cursor parameters
Page.nextCursorisstring | null, but the keyset pagination APIs still takeafter?: string. That makes the documented “passnextCursorback verbatim” flow type-invalid in strict TS. Update the queue cursor params to acceptstring | nulleverywhere these pages are returned (listJobsAfter,listJobsFilteredAfter,listArchivedAfter,deadLettersAfter).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/node/src/types.ts` around lines 178 - 188, Update the cursor parameter types for listJobsAfter, listJobsFilteredAfter, listArchivedAfter, and deadLettersAfter to accept string | null in addition to their existing optional form. Preserve the documented ability to pass Page.nextCursor directly as the after value throughout all APIs returning paginated pages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@sdks/node/src/types.ts`:
- Around line 178-188: Update the cursor parameter types for listJobsAfter,
listJobsFilteredAfter, listArchivedAfter, and deadLettersAfter to accept string
| null in addition to their existing optional form. Preserve the documented
ability to pass Page.nextCursor directly as the after value throughout all APIs
returning paginated pages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6cb9044c-5c73-4d1c-9b77-cabfd38048ab
📒 Files selected for processing (8)
crates/taskito-node/src/queue/inspect.rssdks/java/src/main/java/org/byteveda/taskito/model/Page.javasdks/java/src/main/java/org/byteveda/taskito/worker/WorkerDispatchBridge.javasdks/java/test-support/src/main/java/org/byteveda/taskito/test/InMemoryQueueBackend.javasdks/node/src/index.tssdks/node/src/queue.tssdks/node/src/types.tssdks/node/test/core/pagination.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- sdks/java/src/main/java/org/byteveda/taskito/model/Page.java
- sdks/node/src/index.ts
- sdks/node/test/core/pagination.test.ts
- sdks/java/test-support/src/main/java/org/byteveda/taskito/test/InMemoryQueueBackend.java
- crates/taskito-node/src/queue/inspect.rs
- sdks/java/src/main/java/org/byteveda/taskito/worker/WorkerDispatchBridge.java
- sdks/node/src/queue.ts
Finishes the cross-SDK parity debt: cursor pagination on both shells, the two job listings Node never had, and per-task middleware toggles on Java. With #436 merged, nothing on the parity list is left except the items excluded by design (below).
Node listings it never had
listJobsFilteredandlistArchiveddid not exist. Archived was the sharper gap: a terminal job is moved intoarchived_jobsas it finishes — the live table only ever holds pending/running rows — so a completed job could not be read back at all from this SDK.Cursor pagination (S12)
Core has had the
*_aftermethods since Tier 2 and Python has surfaced them; neither shell did, so a deep listing paid O(offset).listJobsAfter,listJobsFilteredAfter,listArchivedAfter,deadLettersAfter.listJobsAfter,listArchivedAfterwith aPage<T>.next_cursormoves into core besideencode_cursorfirst. It was living in the Python binding, and "a page shorter than the limit is the last one" is part of the cursor contract, not of one shell — leaving it there meant three SDKs writing it three times.Java middleware toggles
disableMiddleware/enableMiddleware/listDisabledMiddleware, over the existing settings and keyed as the other SDKs key it. Read per invocation like Node rather than cached like Python, so a toggle lands on the next job with no restart.The chain is resolved once per invocation and reused for
before/after/onError. Re-reading between them would let a toggle landing mid-job runafteron a middleware whosebeforenever ran — an unbalanced pair authors are entitled to assume cannot happen. There is a test for exactly that.Decisions worth review
default, and they throw.QueueBackendhas an implementor outside this module (InMemoryQueueBackend, 64 overrides), so an abstract method breaks it and every downstream backend. They throw rather than return an empty page: a backend that cannot seek would otherwise look like it had reached the last page. The in-memory backend implements them for real — a default-only method is untested API.Page<T>the smoke never touches gets no reflection entry and fails under--no-fallbackat runtime rather than in CI.Optionas optional but hands backnull. The shell normalizes:nextCursorisnullon the last page, matching the other SDKs. A=== undefinedcheck against the raw value would never have fired.Verification
Every change has a test watched failing with the change removed:
expected 1784…:019f… to be nullaShortPageEndsTheWalkfailsRust 193 tests across default/postgres/redis/native-async, clippy + fmt clean. Node 413 tests, typecheck + biome clean. Java
./gradlew build(tests + spotless + checkstyle) plusplainJavadocJar, which gates publishing rather thanbuild.Still open, by design
list_workflow_runs_after— on the workflow storage trait, reached viawf_storagerather thanstorage; worth its own change alongsidedeadLettersAfterfor Java.dlq:by_task/logs:by_task— blocked on a backfill migration; the index alone would silently miss every pre-upgrade row.Summary by CodeRabbit
nextCursor, plus richer job filtering (status/queue/task, metadata/error substring matching, and created-at ranges).